Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC][asan] Switch from list to DynInitGlobalsByModule #101596

Conversation

vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Aug 2, 2024

The patch just switches container from plain
list of globals, to a map grouped by module.

Prepare for incremental poisoning in #101837

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Aug 2, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Vitaly Buka (vitalybuka)

Changes

Prepare for incremental poisoning, poisoning all
globals for every TU is expensive.


Full diff: https://github.com/llvm/llvm-project/pull/101596.diff

1 Files Affected:

  • (modified) compiler-rt/lib/asan/asan_globals.cpp (+61-28)
diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index cc5308a24fe89..c7dcc47460e83 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -47,8 +47,6 @@ struct DynInitGlobal {
   bool initialized = false;
   DynInitGlobal *next = nullptr;
 };
-typedef IntrusiveList<DynInitGlobal> DynInitGlobals;
-static DynInitGlobals dynamic_init_globals SANITIZER_GUARDED_BY(mu_for_globals);
 
 // We want to remember where a certain range of globals was registered.
 struct GlobalRegistrationSite {
@@ -72,6 +70,22 @@ static ListOfGlobals &GlobalsByIndicator(uptr odr_indicator)
   return (*globals_by_indicator)[odr_indicator];
 }
 
+using DynInitGLobalsByModule =
+    DenseMap<const char *, IntrusiveList<DynInitGlobal>>;
+
+// TODO: Add a NoDestroy helper, this patter is very common in sanitizers.
+static DynInitGLobalsByModule &DynInitGlobals()
+    SANITIZER_REQUIRES(mu_for_globals) {
+  static DynInitGLobalsByModule *globals_by_module = nullptr;
+  if (!globals_by_module) {
+    alignas(alignof(DynInitGLobalsByModule)) static char
+        placeholder[sizeof(DynInitGLobalsByModule)];
+    globals_by_module = new (placeholder) DynInitGLobalsByModule();
+  }
+
+  return *globals_by_module;
+}
+
 ALWAYS_INLINE void PoisonShadowForGlobal(const Global *g, u8 value) {
   FastPoisonShadow(g->beg, g->size_with_redzone, value);
 }
@@ -257,8 +271,8 @@ static void RegisterGlobal(const Global *g) SANITIZER_REQUIRES(mu_for_globals) {
   AddGlobalToList(list_of_all_globals, g);
 
   if (g->has_dynamic_init) {
-    dynamic_init_globals.push_back(new (GetGlobalLowLevelAllocator())
-                                       DynInitGlobal{*g, false});
+    DynInitGlobals()[g->module_name].push_back(
+        new (GetGlobalLowLevelAllocator()) DynInitGlobal{*g, false});
   }
 }
 
@@ -284,20 +298,42 @@ static void UnregisterGlobal(const Global *g)
   }
 }
 
-void StopInitOrderChecking() {
-  if (!flags()->check_initialization_order)
-    return;
-  Lock lock(&mu_for_globals);
-  flags()->check_initialization_order = false;
-  for (const DynInitGlobal &dyn_g : dynamic_init_globals) {
+static void UnpoisonDynamicGlobals(IntrusiveList<DynInitGlobal> &dyn_globals,
+                                   bool mark_initialized) {
+  for (auto &dyn_g : dyn_globals) {
     const Global *g = &dyn_g.g;
+    if (dyn_g.initialized)
+      continue;
     // Unpoison the whole global.
     PoisonShadowForGlobal(g, 0);
     // Poison redzones back.
     PoisonRedZones(*g);
+    if (mark_initialized)
+      dyn_g.initialized = true;
+  }
+}
+
+static void PoisonDynamicGlobals(
+    const IntrusiveList<DynInitGlobal> &dyn_globals) {
+  for (auto &dyn_g : dyn_globals) {
+    const Global *g = &dyn_g.g;
+    if (dyn_g.initialized)
+      continue;
+    PoisonShadowForGlobal(g, kAsanInitializationOrderMagic);
   }
 }
 
+void StopInitOrderChecking() {
+  if (!flags()->check_initialization_order)
+    return;
+  Lock lock(&mu_for_globals);
+  flags()->check_initialization_order = false;
+  DynInitGlobals().forEach([&](auto &kv) {
+    UnpoisonDynamicGlobals(kv.second, /*mark_initialized=*/false);
+    return true;
+  });
+}
+
 static bool IsASCII(unsigned char c) { return /*0x00 <= c &&*/ c <= 0x7F; }
 
 const char *MaybeDemangleGlobalName(const char *name) {
@@ -458,15 +494,16 @@ void __asan_before_dynamic_init(const char *module_name) {
   Lock lock(&mu_for_globals);
   if (flags()->report_globals >= 3)
     Printf("DynInitPoison module: %s\n", module_name);
-  for (DynInitGlobal &dyn_g : dynamic_init_globals) {
-    const Global *g = &dyn_g.g;
-    if (dyn_g.initialized)
-      continue;
-    if (g->module_name != module_name)
-      PoisonShadowForGlobal(g, kAsanInitializationOrderMagic);
-    else if (!strict_init_order)
-      dyn_g.initialized = true;
-  }
+
+  DynInitGlobals().forEach([&](auto &kv) {
+    if (kv.first != module_name) {
+      PoisonDynamicGlobals(kv.second);
+    } else {
+      UnpoisonDynamicGlobals(kv.second,
+                             /*mark_initialized=*/!strict_init_order);
+    }
+    return true;
+  });
 }
 
 // This method runs immediately after dynamic initialization in each TU, when
@@ -479,13 +516,9 @@ void __asan_after_dynamic_init() {
   Lock lock(&mu_for_globals);
   if (flags()->report_globals >= 3)
     Printf("DynInitUnpoison\n");
-  for (const DynInitGlobal &dyn_g : dynamic_init_globals) {
-    const Global *g = &dyn_g.g;
-    if (!dyn_g.initialized) {
-      // Unpoison the whole global.
-      PoisonShadowForGlobal(g, 0);
-      // Poison redzones back.
-      PoisonRedZones(*g);
-    }
-  }
+
+  DynInitGlobals().forEach([&](auto &kv) {
+    UnpoisonDynamicGlobals(kv.second, /*mark_initialized=*/false);
+    return true;
+  });
 }

@vitalybuka vitalybuka requested a review from MaskRay August 2, 2024 01:18
vitalybuka added a commit that referenced this pull request Aug 2, 2024
It's preparation for switching to hash table in #101596.
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@vitalybuka vitalybuka changed the title [NFC][asan] Switch from list to DynInitGLobalsByModule [NFC][asan] Switch from list to DynInitGlobalsByModule Aug 2, 2024
@vitalybuka vitalybuka changed the base branch from users/vitalybuka/spr/main.nfcasan-switch-from-list-to-dyninitglobalsbymodule to main August 3, 2024 15:43
Created using spr 1.3.4
Created using spr 1.3.4
@vitalybuka vitalybuka merged commit 242e8b9 into main Aug 3, 2024
6 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/nfcasan-switch-from-list-to-dyninitglobalsbymodule branch August 3, 2024 17:40
kutemeikito added a commit to kutemeikito/llvm-project that referenced this pull request Aug 4, 2024
* 'main' of https://github.com/llvm/llvm-project:
  [RISCV] Improve hasAllNBitUsers for users of SLLI.
  [RISCV] Invert if conditions in the switch in RISCVDAGToDAGISel::hasAllNBitUsers. NFC
  [Transforms] Construct SmallVector with ArrayRef (NFC) (llvm#101851)
  [RISCV] Capitalize some variable names. NFC
  [sanitizer_common] Fix UnwindFast on SPARC (llvm#101634)
  [builtins] Fix divtc3.c etc. compilation on Solaris/SPARC with gcc (llvm#101662)
  [NFC][asan] Track current dynamic init module (llvm#101597)
  [libc] enable most of the entrypoints on aarch64 (llvm#101797)
  [SandboxIR][Tracker] Track InsertIntoBB (llvm#101595)
  [SCEV] Use const SCEV * explicitly in more places.
  [ELF] Move outputSections into Ctx. NFC
  [ELF] Move ElfSym into Ctx. NFC
  [ELF] Move Out into Ctx. NFC
  [test][asan] Fix the test checks
  [NFC][asan] Switch from list to DynInitGlobalsByModule (llvm#101596)

Signed-off-by: Edwiin Kusuma Jaya <kutemeikito0905@gmail.com>
banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024
banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024
The patch just switches container from plain
list of globals, to a map grouped by module.

Prepare for incremental poisoning in llvm#101837
kstoimenov pushed a commit to kstoimenov/llvm-project that referenced this pull request Aug 15, 2024
The patch just switches container from plain
list of globals, to a map grouped by module.

Prepare for incremental poisoning in llvm#101837
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants